Skip to content

chore: fix requirements.txt parser init & re-enable python tests#404

Merged
Strum355 merged 1 commit into
guacsec:mainfrom
Strum355:nsc/requirements-txt-treesitter-2
Mar 11, 2026
Merged

chore: fix requirements.txt parser init & re-enable python tests#404
Strum355 merged 1 commit into
guacsec:mainfrom
Strum355:nsc/requirements-txt-treesitter-2

Conversation

@Strum355

@Strum355 Strum355 commented Mar 10, 2026

Copy link
Copy Markdown
Member

Description

WIP commenting out of python tests accidentally made it into main : ) re-enabling them and a fix for an issue that came up while testing with vscode extension.

I will need an EA-release of this (on merge to main) to properly test in VSIX, as vsce doesnt support npm link'd dependencies..

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

@Strum355
Strum355 requested a review from ruromero March 10, 2026 17:59
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Fix requirements.txt parser and re-enable Python pip tests

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Fixed requirements.txt parser initialization by removing custom locateFile configuration
• Re-enabled previously commented-out Python pip provider tests
• Restored test coverage for isSupported, stack analysis, and virtual environment scenarios
Diagram
flowchart LR
  A["requirements_parser.js<br/>Parser.init() fix"] --> B["Simplified initialization"]
  C["python_pip.test.js<br/>Uncomment tests"] --> D["isSupported tests"]
  C --> E["Stack analysis tests"]
  C --> F["Virtual env tests"]
  B --> G["Functional parser"]
  D --> G
  E --> G
  F --> G
Loading

Grey Divider

File Changes

1. src/providers/requirements_parser.js 🐞 Bug fix +1/-5

Simplify parser initialization configuration

• Removed custom locateFile() configuration from Parser.init() call
• Simplified initialization to use default web-tree-sitter WASM loading
• Maintains Language.load() for tree-sitter-requirements WASM

src/providers/requirements_parser.js


2. test/providers/python_pip.test.js 🧪 Tests +44/-44

Re-enable all Python pip provider tests

• Uncommented isSupported test suite for requirements.txt file detection
• Re-enabled stack analysis tests using pipdeptree utility
• Restored component analysis tests with pipdeptree utility
• Uncommented virtual environment test suite with multiple scenarios

test/providers/python_pip.test.js


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Async tests not awaited 🐞 Bug ✓ Correctness
Description
The re-enabled pipdeptree tests invoke async helpers without returning/awaiting their Promises, so
Mocha will finish the test before assertions run, producing false positives or unhandled rejections.
Code

test/providers/python_pip.test.js[R75-83]

+		test(`verify requirements.txt sbom provided for stack analysis using pipdeptree utility with scenario ${scenario}`, () => {
+			sharedStackAnalysisTestFlow(testCase, true);
+			// these test cases takes ~2500-2700 ms each pr >10000 in CI (for the first test-case)
+		}).timeout(process.env.GITHUB_ACTIONS ? 30000 : 10000)

-		// test(`verify requirements.txt sbom provided for component analysis using pipdeptree utility with scenario ${scenario}`, () => {
-		// 	sharedComponentAnalysisTestFlow(testCase, true);
-		// 	// these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case)
-		// }).timeout(process.env.GITHUB_ACTIONS ? 15000 : 10000)
+		test(`verify requirements.txt sbom provided for component analysis using pipdeptree utility with scenario ${scenario}`, () => {
+			sharedComponentAnalysisTestFlow(testCase, true);
+			// these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case)
+		}).timeout(process.env.GITHUB_ACTIONS ? 15000 : 10000)
Evidence
The pipdeptree test callbacks are synchronous but call async helpers; those helpers await provider
calls and contain assertions, which will execute after the test has already ended.

test/providers/python_pip.test.js[75-83]
test/providers/python_pip.test.js[11-24]
test/providers/python_pip.test.js[26-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The pipdeptree tests are declared with synchronous callbacks but call `sharedStackAnalysisTestFlow` / `sharedComponentAnalysisTestFlow`, which are `async` and contain awaited provider calls + assertions. Mocha will mark these tests as complete immediately, causing false positives and/or unhandled promise rejections.

### Issue Context
This was introduced by uncommenting the pipdeptree test blocks.

### Fix Focus Areas
- test/providers/python_pip.test.js[75-83]

### Implementation sketch
- Change both tests to `async () =&gt; { await shared... }`.
 - Alternatively: `return sharedStackAnalysisTestFlow(...)` / `return sharedComponentAnalysisTestFlow(...)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Pipdeptree flag ignored 🐞 Bug ✓ Correctness
Description
The re-enabled pipdeptree tests pass a boolean true into opts.TRUSTIFY_DA_PIP_USE_DEP_TREE, but
option parsing only accepts string values, so the controller always uses the default "false" and
never executes the pipdeptree code path.
Code

test/providers/python_pip.test.js[R75-82]

+		test(`verify requirements.txt sbom provided for stack analysis using pipdeptree utility with scenario ${scenario}`, () => {
+			sharedStackAnalysisTestFlow(testCase, true);
+			// these test cases takes ~2500-2700 ms each pr >10000 in CI (for the first test-case)
+		}).timeout(process.env.GITHUB_ACTIONS ? 30000 : 10000)

-		// test(`verify requirements.txt sbom provided for component analysis using pipdeptree utility with scenario ${scenario}`, () => {
-		// 	sharedComponentAnalysisTestFlow(testCase, true);
-		// 	// these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case)
-		// }).timeout(process.env.GITHUB_ACTIONS ? 15000 : 10000)
+		test(`verify requirements.txt sbom provided for component analysis using pipdeptree utility with scenario ${scenario}`, () => {
+			sharedComponentAnalysisTestFlow(testCase, true);
+			// these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case)
Evidence
The tests pass true (boolean) for the flag; getCustom() ignores non-string values in opts, and
python_controller compares the resulting value against the string "true", so the pipdeptree branch
can’t be reached via these tests.

test/providers/python_pip.test.js[75-82]
test/providers/python_pip.test.js[11-18]
test/providers/python_pip.test.js[26-39]
src/tools.js[37-42]
src/providers/python_controller.js[189-202]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`TRUSTIFY_DA_PIP_USE_DEP_TREE` is passed as a boolean in test opts, but `getCustom()` only reads string values from `opts`. As a result, pipdeptree tests never enable the pipdeptree path.

### Issue Context
The provider uses string comparisons (`!== &quot;true&quot;`), so the flag must be a string when passed via `opts`.

### Fix Focus Areas
- test/providers/python_pip.test.js[11-18]
- test/providers/python_pip.test.js[26-39]
- test/providers/python_pip.test.js[75-82]

### Implementation sketch
- In both shared flows, set:
 - `const opts = { TRUSTIFY_DA_PIP_USE_DEP_TREE: usePipDepTreeUtility ? &quot;true&quot; : &quot;false&quot; }`
- Keep the call sites as-is (passing boolean) or also convert call sites to pass strings for clarity.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Env var not restored 🐞 Bug ⛯ Reliability
Description
The re-enabled virtual-environment tests set process.env.TRUSTIFY_DA_PYTHON_VIRTUAL_ENV = "true"
and never restore it, making subsequent tests/order run with an unintended virtualenv behavior.
Code

test/providers/python_pip.test.js[R95-101]

+			// load the expected sbom stack analysis
+			let expectedSbom = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/expected_stack_sbom.json`,).toString()
+			process.env["TRUSTIFY_DA_PYTHON_VIRTUAL_ENV"] = "true"
+			// process.env["TRUSTIFY_DA_DEBUG"] = "true"
+			expectedSbom = JSON.stringify(JSON.parse(expectedSbom), null, 4)
+			// invoke sut stack analysis for scenario manifest
+			let providedDataForStack = await pythonPip.provideStack(`test/providers/tst_manifests/pip/${testCase}/requirements.txt`)
Evidence
The test mutates a global environment variable; the provider reads that variable to decide whether
to create a virtual environment, and the suite’s teardown only restores fake timers, not the env
var.

test/providers/python_pip.test.js[95-99]
test/providers/python_pip.test.js[117-117]
src/providers/python_pip.js[158-163]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The virtualenv test suite sets `process.env.TRUSTIFY_DA_PYTHON_VIRTUAL_ENV` to &quot;true&quot; but never restores it. This leaks global state into other tests and makes the test run order-dependent.

### Issue Context
`python_pip` reads `TRUSTIFY_DA_PYTHON_VIRTUAL_ENV` via `getCustom(...)`, so the setting can be provided through `opts` instead of `process.env`.

### Fix Focus Areas
- test/providers/python_pip.test.js[94-115]
- src/providers/python_pip.js[158-163]

### Implementation sketch
Option A (preferred):
- Remove the `process.env[...] = &quot;true&quot;` line.
- Call `pythonPip.provideStack(..., { TRUSTIFY_DA_PYTHON_VIRTUAL_ENV: &quot;true&quot; })`.

Option B:
- Capture previous value before setting, and restore it in `afterEach`/`afterAll` (including deleting the key if it was previously unset).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@Strum355
Strum355 enabled auto-merge (squash) March 10, 2026 18:02
Comment thread test/providers/python_pip.test.js Outdated
Comment thread test/providers/python_pip.test.js Outdated
Comment thread test/providers/python_pip.test.js Outdated
@Strum355
Strum355 force-pushed the nsc/requirements-txt-treesitter-2 branch 2 times, most recently from 8b0f978 to 2280b2c Compare March 10, 2026 18:09
Comment thread test/providers/python_pip.test.js
ruromero
ruromero previously approved these changes Mar 10, 2026
@Strum355
Strum355 force-pushed the nsc/requirements-txt-treesitter-2 branch from 2280b2c to a8c6f8e Compare March 11, 2026 13:34
@Strum355
Strum355 requested a review from ruromero March 11, 2026 15:07
@Strum355
Strum355 merged commit 6549d2a into guacsec:main Mar 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants